Repository navigation
Conversation
c0a0031 to
91a5ba0
Compare
There was a problem hiding this comment.
Thanks for splitting this up. Trends reads well and the Statistics parity work is careful. Before this goes further I'd like to shrink it and remove the overlap with Statistics.
Replace Statistics. Trends should replace Statistics in this PR rather than sit beside it in Settings. Two screens answering the same question will confuse people, and the parity commit already covers what Statistics shows. Please remove the Statistics entry and the screen as part of this change.
Duplicated figures. Several numbers appear two or three times on one page:
- Insulin per day and the basal/bolus split are in the Summary tile, the Basal and bolus card, and the Insulin per day chart's average line and legend. Could Summary stay focused on glucose (plus carbs and boluses), and Basal and bolus fold into the Insulin per day card, with Scheduled and Manual/Automatic under the chart?
- Loop active in Summary repeats the Loop status card, and it covers a different window from the period's other figures. It's clearer only in Loop status, beside its coverage note.
- "Usually X U by HH:MM" appears twice. One place is enough.
Aim lines. The Aim sentences, especially the six-target one under Time in range, are hard to scan. Showing each band's target in its row, or marking a figure that misses its target, would say the same with less text.
Colors. The PR adds 17 named colors, and most could reuse what Loop already has. glucose-ink, accent-ink, insulin-ink, bolus and carbs-ink are variants of the existing glucose, accent, insulin and carbs colors. agp-inner, agp-outer, target-band, basal and basal-edge can be those colors at reduced opacity, with no new color sets. ink-muted can be .secondary. The five range-* colors duplicate GlucoseBand.agpColor in Statistics (TimeInRangeBar.swift). Since Trends replaces Statistics, keep one time-in-range palette; your dark-mode variants are an improvement, so move them there. control-thumb goes away with a system segmented Picker.
Scope. Could the day page, Gaps and Export come as follow-up parts? Export in particular produces a shareable, clinical-looking report that deserves its own review.
Open/closed loop history. Loop keeps 90 days of data (Bundle.main.localCacheDuration, LOOP_LOCAL_CACHE_DURATION_DAYS), and localHistoryStart already uses it. Rather than a new UserDefaults record (LoopModeHistory, 100 days), please read closed/open loop from data Loop already stores. StoredSettings carries dosingEnabled with a date, and SettingsStore keeps those records for the cache duration. The provider already uses getStoredSettings for target history. If that doesn't work out, a new record should follow the other stores' design (Core Data in the cache store, expiring with localCacheDuration) rather than live in UserDefaults.standard. It's also worth checking that every closed-loop toggle path saves settings, so the history isn't missing changes.
"Loop keeps 7 days". Two strings hard-code this: the Loop status footnote and the day page's Apple Health note. The provider's window is localHistoryStart, which is the cache duration, 90 days by default, so the text is wrong for most builds. It should follow the cache duration rather than hard-code a number.
Older periods. For days beyond Loop's local history, how does the screen show that the Manual/Automatic split and the scheduled basal are partial or estimated?
Trends reviews one period at a time (a day, 7, 14, 30 or 90 days) on one screen: summary tiles (average with SD and CV, GMI, insulin, carbs and boluses per day, Loop active), time in range, the daily pattern (AGP), basal and bolus, insulin per day and Loop status with its gaps. A one-day period leads with that day's timeline. Tapping a day opens a day page with the insulin timeline and active insulin and carbs, stepping a day at a time. The period exports as a PDF report or a CSV with one row per day. Inside Loop's 7 days the figures come from Loop's own stores, so manual and automatic boluses, temp basals, historical targets and presets are known. Older days come from Apple Health. Closed and open loop are recorded from launch, because Loop's automation history only covers 36 hours. Trends opens from Settings, beside Statistics, as a sheet with its own navigation and a Done button. Statistics and Current Setup are unchanged. This is part 1 of 6 splitting LoopKit#2621. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP7ynB62LNuuYMy3yvH
Trends now carries everything Statistics shows that it lacked, so it can take Statistics' place: - Consensus goals: average under 8.6 mmol/L (154 mg/dL), GMI under 7% and CV 36% or less under the summary tiles; time in range over 70%, under 25% above and 4% below, under 5% very high and 1% very low, and CGM active over 70% on the Time in range card. - Scheduled basal under Basal in Basal and bolus: the schedule that applied over the same days (the stored profile with presets), or today's schedule where the profile history doesn't reach. - "Usually X U by HH:MM": today's insulin against the median by this time over the last 14 days, from InsulinDeliveryHistory as on Statistics. Under Total so far on today's Day view, and under the Insulin per day chart on longer periods ending today. - "CGM data covers 5 of 14 days." under Summary when readings start partway through the period. - Typed-in (fingerstick) readings are left out of every figure, as on Statistics. The day timeline still draws them. Each new line reuses a pattern already on the screen: the Aim footnote, the indented line under Bolus, and the muted footnote. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP7ynB62LNuuYMy3yvH
Removes the Statistics screen and its Settings entry, and puts Trends where it was. GlucoseBand's display extension, SetupComparisonCharts, GlucoseStatistics and InsulinDeliveryHistory stay, because Current Setup uses them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
A one-day period still leads with its timeline but no longer opens a day page. The removed code, strings and tests come back in their own parts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Summary keeps glucose, carbs and boluses. Basal and bolus fold into the Insulin per day card, Loop active is only in Loop status, and "usually by now" appears once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Each summary tile carries its aim and each time-in-range band shows its target in its row. Misses are not coloured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Removes the 17 colour sets. Marks use the existing glucose, insulin, carbs and accent colours, text uses primary and secondary, and GlucoseBand.agpColor gains dark variants. The period selector is a system segmented Picker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Loop status reads dosingEnabled from SettingsStore, including the record before the period, instead of a UserDefaults record. A manual temp basal counts as open loop, as on the home screen. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
The Loop status note uses localCacheDuration instead of 7 days. Scheduled basal and the manual and automatic split cover only days Loop has itself, with a day count, and boluses of unknown kind are no longer counted as manual. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Time in range, basal and bolus, and Loop status rows stack at accessibility sizes instead of spilling or breaking words. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Moves the smooth-curve helper into SetupComparisonCharts, its remaining user. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Reads Apple Health for days before Loop's first local records, per kind of data, without overlap. Classifies each bolus from its own record and shows unknown. Always shows GMI with its IFCC value, with a note when the period is short or coverage is low. Counts time-in-range hours over CGM-covered time, restores dark-mode tile backgrounds and matches the Settings chevron. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Limits the first-record scan to each week, treats logged injections as manual, lets a recorded manual temp basal end win, refreshes cached first records when stores change and lets the Boluses detail wrap. Adds one bolus colour set, so bolus stands apart from basal in light mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9
e613641 to
150196b
Compare
|
Thanks, Pete. I've taken almost all of this, one commit per point (listed in the description). Point by point: 1. Replace Statistics
Done. The Statistics entry and screen are removed: Everything Current Setup shares with Statistics stays: the 2. Duplicated figures
All three are done:
3. Aim lines
The Aim sentences are gone. Each target sits beside its figure, muted:
High and Low read "< 25% above range" and "< 4% below range", because those consensus targets cover both bands on that side, and a bare "< 25%" beside High alone would mislead. Misses aren't coloured; red stays for warnings. 4. Colours
16 of the 17 are gone:
I'd like to keep one: a darker 5. Scope
Done. The day page, the Gaps list and Export (PDF and CSV) are out of this PR, with their strings, code and tests. A one-day period still leads with that day's timeline, but it no longer opens a day page. 6. Open/closed loop history
Done:
7. "Loop keeps 7 days"Fixed. The Loop status footnote reads "Covers …, from the start of Loop's records, which it keeps for 90 days", with the number from 8. Older periods
Before, it didn't: Scheduled used today's schedule for older days as if it were fact. Now:
While checking this, I also found that days inside the cache window but before Loop's first records came up empty. Trends now reads Apple Health for those, with the boundary per kind of data at the later of the cache start and Loop's first record, and no overlap. Also fixed while testing
Deferred
🤖 Generated with Claude Code |
Nutcasey
left a comment
There was a problem hiding this comment.
Codex has reviewed PR #2637 using code-reviewer-cx, against next-dev f7b08cad, including the full diff and Pete's existing feedback. Codex has fixed all outstanding high-confidence issues found in this review. No further findings at or above 80/100 confidence remain.
Critical (90–100 confidence)
- 98/100 — Schedule history extended into unknown time. LoopHistoryProvider.swift:107: the historical schedule queries backfilled the first saved profile and skipped records without a profile. Trends could therefore infer basal delivery and draw targets during unknown time. Fixed in 0081d73c: reporting uses each saved setting only for its recorded interval, including the setting before the requested start. A real-
SettingsStoreregression catches both leading unknown time and missing profiles; restoring the old queries produces 5 U instead of 3 U, and the corrected implementation passes.
Important (80–89 confidence)
- 89/100 — Unreadable chart axes at accessibility text sizes. TrendsComponents.swift:732: the 90-day chart's dates overlapped at AX XXXL. Fixed in 596606a2: fewer axis labels, correctly anchored end dates, and stacked card titles/subtitles. The AGP and day timeline also reduce their axis labels at accessibility sizes. Verified on the
loopsimulator, including a before/after check of the insulin chart.
Verification
- App build and
build-for-testingsucceeded, using the separate #2652 watch build fix locally; that fix is excluded from these commits. - Operated Settings → Trends, all five periods, previous/next navigation, older Apple Health data, empty periods, light/dark appearance and AX XXXL; Current Setup still opens.
- 457 selected LoopTests: 455 passed, 2 failed.
LoopDataManagerTests28/28,LoopHistoryMathTests32/32,GlucoseStatisticsTests12/12. - The two failures are the already documented
BolusEntryViewModelTests.testCarbEntryDateAndAbsorptionTimeStringand…String2locale assertions (12:00 PMversus12:00). They also appear in the earlier pre-review run. HealthKit-hostedDeviceDataManagerTestswere excluded; this is not a claim that the complete hosted suite passes. git diff --check, catalogue JSON and new-string placeholder checks pass. Existing translations are unchanged.
The description now includes these fixes and the final test results. The PR is open and ready for human review.










Part 1 of the series splitting #2621. Updated after Pete's review (3 November 2026): Trends now replaces Statistics, each figure appears once, and the day page, Gaps and Export have moved to follow-up parts.
Trends reviews one period at a time: a day, 7, 14, 30 or 90 days. It opens from Settings → Trends, where Statistics was, as a sheet with a Done button.
Where the figures come from
StoredSettings.dosingEnabled, whichSettingsStorekeeps for the cache duration, including the record before the period. A manual temp basal counts as open loop, as on the home screen since Show the loop as open while a manual temp basal is running #2649, from the temp basal's own record and its end or cancellation.Colours
One new colour set. Marks use Loop's existing
glucose,insulin,carbsandaccent; text uses.primaryand.secondary; the AGP bands, target band and basal are those colours at reduced opacity. Time in range usesGlucoseBand.agpColor, which gains dark-mode variants (Current Setup picks these up). The period picker is a system segmentedPicker.The one addition is a darker
bolus(light#A9480A, dark#FFA347). Plaininsulinbolus beside fadedinsulinbasal was too close to tell apart in use:insulinis only 2.3:1 on white, so no lighter tint of it reaches 3:1 against it. With the darker bolus and a card-coloured gap where bolus sits on basal, they're 4.7:1 in light mode and 4.4:1 in dark.Commits
The first two are the original Trends and Statistics-parity commits, rebased onto
next-devf7b08cad. The review changes follow, one commit per point so each can be reviewed on its own:Screenshots
Annotated, with before and after where something was removed. From the
loopsimulator's demo data; its seeded boluses before 26 October have no manual/automatic flag, which is why much of the 14-day bolus shows as unknown.Also: Current Setup still working, Boluses tile with unknown.
Testing
On the
loopsimulator (iOS 27.0, Pump and CGM Simulators, 90 days of history), onnext-devf7b08cad:Selected LoopTests: 457 run, 455 pass. The two failures are
BolusEntryViewModelTests.testCarbEntryDateAndAbsorptionTimeStringand…String2, which fail the same way on plainnext-dev("12:00 PM" against "12:00").LoopHistoryMathTestscovers the boundary, manual temp basal, GMI and classification rules. The Codex follow-up adds a real-SettingsStoreregression for schedule start dates, missing profiles and the preceding setting; restoring the old queries reproduces 5 U instead of the correct 3 U.LoopDataManagerTestspasses 28/28 andLoopHistoryMathTestspasses 32/32. HealthKit-hostedDeviceDataManagerTestswere not run.Building
next-devcurrently needs #2652 (WatchApp still links G7SensorKit after #2645). It isn't part of this PR.Strings. New keys have translations in Loop's languages; native-speaker review is welcome. Statistics' strings and those only the moved screens used are removed. Shared keys gain missing translations; existing translations are unchanged. JSON and format-placeholder checks pass.
Deferred to later parts
The series
#2621 is split into PRs in the order Pete suggested: statistics and Trends first, then the Logbook, then the home screen. Add carbs, the widget and Live Activity, and the appearance options follow as their own parts, and the day page with Gaps, and Export, now come as two more. Each part builds on the one before it and targets
next-dev, so a PR shows the earlier parts' commits too. #2621 stays open as the combined reference until the series lands.Merging. With squash merges, each next PR needs a rebase onto
next-devonce the previous one lands. I'll rebase each PR as the one before it merges.Strings. Each part adds only its own catalogue entries, with their translations in Loop's languages. GitHub's line diff of
Localizable.xcstringscan show deletions where Xcode's ordering interleaves new keys.🤖 Generated with Claude Code
https://claude.ai/code/session_01QHnVP6NkV9kiyLRBAeMMt9